Add PECO power outage counter integration #65194

IceBotYT · 2022-01-29T18:41:34Z

Proposed change

I want to add a new PECO integration that counts the power outages in a user's area.
PECO is the energy company for the Philadelphia and Philadelphia metro.
Learn more here

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

Link to documentation pull request: Add page for PECO integration home-assistant.io#21392

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

This reverts commit 38309c5.

This reverts commit aeabdd3.

IceBotYT · 2022-01-29T18:47:55Z

I had to revert the first two commits because I closed the other PR for that integration and forgot to push the delete commits.
Please remove the nws_alerts integration label!
Sorry! :/

IceBotYT · 2022-01-29T22:22:07Z

Want me to fix the "method could be a function error"?

IceBotYT · 2022-01-29T23:20:37Z

Anything I need to do at the moment @farmio?

farmio · 2022-01-30T06:16:53Z

Want me to fix the "method could be a function error"?

Yes sure CI should be green.
Also add all untested files to .coveragerc. Except for config_flow.py - it should have 100% test coverage.

Then you will have to be patient until a member reviews your code. Consider adding tests or type annotations meanwhile - this makes it easier to review.
And please don't tag people unless you have to - same for discord.

tests/components/peco/test_config_flow.py

… county

IceBotYT · 2022-01-30T14:40:09Z

Ah I see. It's looking for the wrong error. Didn't know voluptuous did that!

IceBotYT · 2022-01-30T14:55:04Z

Idk if you get e-mails for new commits, but I fixed the MultipleValid error and added some more type definitions

IceBotYT · 2022-01-30T18:39:01Z

????????????
I try to commit with regular dict and it says to use typing.Dict, and now the CI here is saying to use dict instead????

I guess I'll use regular dict if that's what pylint wants

IceBotYT · 2022-01-30T18:50:59Z

Here's a screenshot from the commit command

farmio

If you are serious about typing try adding your integration to .strict_typing https://github.com/home-assistant/core/blob/dev/.strict-typing and run mypy again.

homeassistant/components/peco/sensor.py

farmio · 2022-01-30T19:03:28Z

Type hinting generics in standard collections was added in Python 3.9. Maybe you are using outdated mypy or run it with older Python.

Idk if you get e-mails for new commits

Good grief, I'm not 😱
If you encounter issues with integration development check out the dedicated channels at the HA discord server. And please always run checks locally before pushing - this saves a lot of time for you and a reviewer.

gjohansson-ST

Added a few comments.
Seems also the deletion of translation files from the update integration came into this PR?!

homeassistant/components/peco/__init__.py

homeassistant/components/peco/sensor.py

homeassistant/components/peco/strings.json

homeassistant/components/peco/translations/en.json

IceBotYT · 2022-03-15T21:24:57Z

Done!

farmio

Some final suggestions. Other than that I think its ready to go 👍

as gjohansson-ST mentioned: Seems the deletion of translation files from the update integration came into this PR.

homeassistant/components/peco/config_flow.py

homeassistant/components/peco/sensor.py

ggravlingen · 2022-03-20T19:34:49Z

A much better title and description than before of this PR 🌟

IceBotYT · 2022-03-20T19:36:57Z

I didn't change any dependencies, how do I fix the conflict?

ggravlingen · 2022-03-20T19:43:56Z

I didn't change any dependencies, how do I fix the conflict?

Try pinning the version here to the same version as in home assistant and see if it’s helps: https://github.com/IceBotYT/peco-outage-api/blob/9850a9682ca90d8b8ec1d5c33c5e26020b7e306e/setup.cfg#L23

ggravlingen · 2022-03-20T19:45:15Z

It’s generally considered good practice to pin versions of dependencies to avoid unwanted updates of dependencies’ dependencies, and to avoid security issues.

gjohansson-ST · 2022-03-20T20:04:52Z

I didn't change any dependencies, how do I fix the conflict?

The pip errors are not related to this PR (I have same on one of my PR's). I think someone from core team needs to fix something.

ggravlingen · 2022-03-20T20:14:39Z

I didn't change any dependencies, how do I fix the conflict?

The pip errors are not related to this PR (I have same on one of my PR's). I think someone from core team needs to fix something.

I still recommend pinning the version though.

…he outage map, as noted by farmio

Merging to fix dependency conflict upstream to make CI pass

farmio

Looks nice! 👍

gjohansson-ST

👍

frenck

Hi there @IceBotYT 👋

Please address these comments in a new PR.

Thanks! 👍

homeassistant/components/peco/sensor.py

homeassistant/components/peco/strings.json

tests/components/peco/test_sensor.py

homeassistant/components/peco/sensor.py

homeassistant/components/peco/config_flow.py

MartinHjelmare

Please address the comments in a new PR. Thanks!

homeassistant/components/peco/sensor.py

IceBotYT and others added 4 commits January 24, 2022 22:22

Create a new NWS Alerts integration

38309c5

Create a new NWS Alerts integration

aeabdd3

Create new PECO integration

03c9141

Remove empty keys

34d051a

homeassistant added the cla-signed label Jan 29, 2022

probot-home-assistant bot added has-tests integration: nws_alerts new-integration integration: peco labels Jan 29, 2022

project-bot bot added this to Needs review in Dev Jan 29, 2022

This was referenced Jan 29, 2022

Adding PECO logo and icon home-assistant/brands#3124

Merged

Add page for PECO integration home-assistant/home-assistant.io#21392

Merged

IceBotYT added 2 commits January 29, 2022 18:45

Revert "Create a new NWS Alerts integration"

906e4f8

This reverts commit 38309c5.

Revert "Create a new NWS Alerts integration"

15e8d40

This reverts commit aeabdd3.

Fix test with new mock data

f646cd9

farmio removed the integration: nws_alerts label Jan 29, 2022

Add init and sensor to .coveragerc and more tests for config flow

ced625f

farmio reviewed Jan 30, 2022

View reviewed changes

tests/components/peco/test_config_flow.py Outdated Show resolved Hide resolved

Small fixes and replacing patch with pytest.raises in testing invalid…

fe96df9

… county

Add type defs and fix test_config_flow to use MultipleValid instead

1a3de51

farmio reviewed Jan 30, 2022

View reviewed changes

Remove unused variable from the peco sensor

7265ff2

IceBotYT requested a review from farmio March 15, 2022 20:25

gjohansson-ST requested changes Mar 15, 2022

View reviewed changes

Add rounding to percentage, and extra clean-up

1704d36

IceBotYT changed the title ~~Create new PECO integration~~ Create new PECO power outage counter integration Mar 18, 2022

farmio reviewed Mar 19, 2022

View reviewed changes

IceBotYT added 2 commits March 20, 2022 13:44

Final suggestions from @farmio

b83e77f

Update SCAN_INTERVAL to be a little bit faster

fd11fb6

IceBotYT added 2 commits March 20, 2022 20:46

Change the SCAN_INTERVAL to be somewhat near the update interval of t…

53d2625

…he outage map, as noted by farmio

Merge remote-tracking branch 'upstream/dev' into peco-outage-count

9491e60

Merging to fix dependency conflict upstream to make CI pass

farmio approved these changes Mar 20, 2022

View reviewed changes

Dev automation moved this from Review in progress to Reviewer approved Mar 20, 2022

gjohansson-ST approved these changes Mar 21, 2022

View reviewed changes

IceBotYT added 2 commits March 21, 2022 21:36

Fix config_flows.py to new method

a8e9218

New UpdateCoordinator typing

49c180b

farmio changed the title ~~Create new PECO power outage counter integration~~ Add PECO power outage counter integration Mar 21, 2022

farmio merged commit a43505a into home-assistant:dev Mar 21, 2022

Dev automation moved this from Reviewer approved to Done Mar 21, 2022

IceBotYT deleted the peco-outage-count branch March 21, 2022 23:22

frenck reviewed Mar 22, 2022

View reviewed changes

IceBotYT restored the peco-outage-count branch March 22, 2022 16:48

github-actions bot locked and limited conversation to collaborators Mar 23, 2022

MartinHjelmare reviewed Jun 13, 2022

View reviewed changes

homeassistant/components/peco/sensor.py Show resolved Hide resolved

homeassistant/components/peco/sensor.py Show resolved Hide resolved

homeassistant/components/peco/sensor.py Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PECO power outage counter integration #65194

Add PECO power outage counter integration #65194

IceBotYT commented Jan 29, 2022 •

edited

IceBotYT commented Jan 29, 2022 •

edited

IceBotYT commented Jan 29, 2022

IceBotYT commented Jan 29, 2022

farmio commented Jan 30, 2022

IceBotYT commented Jan 30, 2022

IceBotYT commented Jan 30, 2022

IceBotYT commented Jan 30, 2022 •

edited

IceBotYT commented Jan 30, 2022

farmio left a comment

farmio commented Jan 30, 2022

gjohansson-ST left a comment

IceBotYT commented Mar 15, 2022

farmio left a comment

ggravlingen commented Mar 20, 2022

IceBotYT commented Mar 20, 2022

ggravlingen commented Mar 20, 2022

ggravlingen commented Mar 20, 2022

gjohansson-ST commented Mar 20, 2022

ggravlingen commented Mar 20, 2022

farmio left a comment

gjohansson-ST left a comment

frenck left a comment

MartinHjelmare left a comment

Add PECO power outage counter integration #65194

Add PECO power outage counter integration #65194

Conversation

IceBotYT commented Jan 29, 2022 • edited

Proposed change

Type of change

Additional information

Checklist

IceBotYT commented Jan 29, 2022 • edited

IceBotYT commented Jan 29, 2022

IceBotYT commented Jan 29, 2022

farmio commented Jan 30, 2022

IceBotYT commented Jan 30, 2022

IceBotYT commented Jan 30, 2022

IceBotYT commented Jan 30, 2022 • edited

IceBotYT commented Jan 30, 2022

farmio left a comment

Choose a reason for hiding this comment

farmio commented Jan 30, 2022

gjohansson-ST left a comment

Choose a reason for hiding this comment

IceBotYT commented Mar 15, 2022

farmio left a comment

Choose a reason for hiding this comment

ggravlingen commented Mar 20, 2022

IceBotYT commented Mar 20, 2022

ggravlingen commented Mar 20, 2022

ggravlingen commented Mar 20, 2022

gjohansson-ST commented Mar 20, 2022

ggravlingen commented Mar 20, 2022

farmio left a comment

Choose a reason for hiding this comment

gjohansson-ST left a comment

Choose a reason for hiding this comment

frenck left a comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

IceBotYT commented Jan 29, 2022 •

edited

IceBotYT commented Jan 29, 2022 •

edited

IceBotYT commented Jan 30, 2022 •

edited